Added LOGO track#328
Conversation
✅ Deploy Preview for cozy-marzipan-abb3b4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| const end = Math.round(this["display-end"] ?? this.length ?? start); | ||
|
|
||
| if (end < start) return; |
There was a problem hiding this comment.
When the user doesn't set display-end the position mixin defaults it to a special value, -1, meaning "show everything to the end" (it's named WHOLE_SEQ in nightingale-new-core).
Because ?? only falls back on null/undefined (not -1), end becomes -1, and the if (end < start) return; on the next line bails out so nothing is drawn. withZoom already handles this, we just need to do the same when choosing end:
const rawEnd = this["display-end"];
const end = Math.round(rawEnd && rawEnd !== -1 ? rawEnd : (this.length ?? start));There was a problem hiding this comment.
Should be resolved, please check
| // Most-conserved nucleotide at the bottom of the stack | ||
| letters.sort((a, b) => b.heightFraction - a.heightFraction); |
There was a problem hiding this comment.
I noticed on other logo plots eg https://weblogo.threeplusone.com/examples.html they have the largest letters on top while this PR has them on the bottom. I just wanted to doublecheck which way you want it?
There was a problem hiding this comment.
updated to set the largest letter at the top.
|
|
||
| @customElementOnce("nightingale-logo-track") | ||
| export default class NightingaleLogoTrack extends withManager( | ||
| withZoom(withHighlight(NightingaleElement)) |
There was a problem hiding this comment.
The component mixes in withHighlight currently doesn't draw the highlighted region or reacts to those attributes. withHighlight on its own only tracks which region is highlighted and then each component then needs to implement how the highlight is rendered.
There's a ready-made mixin for exactly this: withSVGHighlight (see how nightingale-variation uses it). It gives you createHighlightGroup() and a working updateHighlight(). It would involve calling createHighlightGroup() in createTrack() and updateHighlight() inside refresh(), and the highlight rectangle should then work.
| "types": "dist/index.d.ts", | ||
| "scripts": { | ||
| "build": "rollup --config ../../rollup.config.mjs", | ||
| "test": "../../node_modules/.bin/jest --config ../../jest.config.js ./tests/*" |
There was a problem hiding this comment.
The test script runs jest against ./tests/*, but there's no tests folder in the package yet, so yarn test here doesn't actually run anything.
There was a problem hiding this comment.
Should be resolved, please check
| parts.push( | ||
| `<text ` + | ||
| `transform="translate(${centerX.toFixed(2)},${baselineY.toFixed(2)}) scale(${scaleX.toFixed(3)},${scaleY.toFixed(3)})" ` + | ||
| `text-anchor="middle" ` + | ||
| `font-family="Arial,Helvetica,sans-serif" font-size="${FONT_SIZE}" font-weight="bold" ` + | ||
| `fill="${color}">${item.nucleotide}</text>` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Draw margin overlays to mask letters that extend into margin areas | ||
| parts.push( | ||
| `<rect x="0" y="0" width="${marginLeft}" height="${height}" fill="${marginColor}" pointer-events="none"/>`, | ||
| `<rect x="${width - marginRight}" y="0" width="${marginRight}" height="${height}" fill="${marginColor}" pointer-events="none"/>`, | ||
| `<rect x="${marginLeft}" y="0" width="${width - marginLeft - marginRight}" height="${marginTop}" fill="${marginColor}" pointer-events="none"/>`, | ||
| `<rect x="${marginLeft}" y="${height - marginBottom}" width="${width - marginLeft - marginRight}" height="${marginBottom}" fill="${marginColor}" pointer-events="none"/>` | ||
| ); |
There was a problem hiding this comment.
This assembles the SVG as one big string and assigns it with innerHTML on every refresh. It works, but two things:
refresh()runs on every zoom/pan frame, and this throws away and re-parses the whole column DOM each time which takes bit of time.- The other tracks (have a look at
nightingale-linegraph-track) build and update their SVG with d3's "data join" —selectAll(...).data(...).join("text")which only touches the elements that actually changed and keeps the code consistent across the repo.
Moving to a d3 join would make it both faster and more in keeping with the rest of the codebase.
There was a problem hiding this comment.
Should be resolved, please check
|
|
||
| // Draw margin overlays to mask letters that extend into margin areas | ||
| parts.push( | ||
| `<rect x="0" y="0" width="${marginLeft}" height="${height}" fill="${marginColor}" pointer-events="none"/>`, |
There was a problem hiding this comment.
The withMargin mixin you're already using provides a renderMarginOnGroup(group) helper that draws exactly the four rectangles margin for you (and reuses them rather than recreating them). nightingale-linegraph-track shows the usage. Switching to it is less code to maintain, and you'll stay in sync automatically if the shared margin behaviour ever changes.
There was a problem hiding this comment.
Should be resolved, please check
| const NUCLEOTIDE_COLORS: Record<string, string> = { | ||
| A: "#00CC00", | ||
| C: "#0000CC", | ||
| G: "#FFB300", | ||
| U: "#CC0000", |
There was a problem hiding this comment.
Would you consider extending to work with amino acids too?
There was a problem hiding this comment.
Sure! Updated to add this capability
✅ Deploy Preview for cozy-marzipan-abb3b4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
2 participants
No description provided.